Skip to content

Block server side restore in save&restore - #4021

Open
georgweiss wants to merge 3 commits into
masterfrom
CSSTUDIO-3817
Open

Block server side restore in save&restore#4021
georgweiss wants to merge 3 commits into
masterfrom
CSSTUDIO-3817

Conversation

@georgweiss

@georgweiss georgweiss commented Nov 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Restore of snapshots from the service may pose a security issue as it may circumvent EPICS access control. As discussed in recent codathon meeting, the server side restore feature should be optional.

This PR adds a property controlling the presence of the restore endpoint, which by default will be disabled. To enable the endpoints, service must be launched with property serverRestoreAllowed=true.

The info endpoint (default endpoint) has been updated to return a ServerInfo object wrapping the data that might be of interest, e.g. if server side restore is enabled and statistics on the Elastic indices (see below).

  • Testing:

    • The feature has automated tests
    • Tests were run
    • If not, explain how you tested your changes

  • Documentation:

    • The feature is documented
    • The documentation is up to date
    • Release notes:
      • Added an entry if the change is breaking or significant
      • Added an entry when adding a new feature

Info endpoint response example:

{
  "serviceVersion": "7.0.0-SNAPSHOT",
  "serverRestoreEnabled": false,
  "elasticInfo": {
    "connectStatus": "Connected",
    "version": "8.19.16",
    "indexStats": [
      {
        "indexName": "saveandrestore_tree",
        "documentCount": 21701,
        "deletedDocuments": 39,
        "indexSize": 4401174
      },
      {
        "indexName": "saveandrestore_configuration",
        "documentCount": 3899,
        "deletedDocuments": 3,
        "indexSize": 12422529
      },
      {
        "indexName": "saveandrestore_snapshot",
        "documentCount": 14144,
        "deletedDocuments": 0,
        "indexSize": 842965109
      },
      {
        "indexName": "saveandrestore_composite_snapshot",
        "documentCount": 1092,
        "deletedDocuments": 1,
        "indexSize": 681578
      },
      {
        "indexName": "saveandrestore_filter",
        "documentCount": 17,
        "deletedDocuments": 0,
        "indexSize": 17574
      }
    ]
  }
}

@georgweiss
georgweiss requested a review from shroffk November 12, 2026 16:35
jacomago
jacomago reviewed Nov 13, 2026
View reviewed changes

@jacomago jacomago left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure why this is a configuration property rather than an access role?

If it is to be configuration should it be start time or runtime? I.e. should it be a flag that an admin can reset, for example turn off restores during maintenance, turn on during operations?

@georgweiss

georgweiss commented Nov 13, 2026

Copy link
Copy Markdown
Collaborator Author

Server side restore was added for a scenario where the Phoebus client is unable to perform write operations. But such a limitation may be intentional (e.g. ESS), so server side restore may be unwanted. On top of that, users may not know how to use the current UI where both alternatives are presented.

During the codathon it was decided to make the server side restore a "global" option. Additionally the Phoebus UI would - based on the presence of the restore endpoints - be able to decide how to present this in the UI.

So no, dynamic or role-based behavior is not in scope.

@jacomago

jacomago commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Server side restore was added for a scenario where the Phoebus client is unable to perform write operations. But such a limitation may be intentional (e.g. ESS), so server side restore may be unwanted. On top of that, users may not know how to use the current UI where both alternatives are presented.

During the codathon it was decided to make the server side restore a "global" option. Additionally the Phoebus UI would - based on the presence of the restore endpoints - be able to decide how to present this in the UI.

So no, dynamic or role-based behavior is not in scope.

Ah right, it's not all restore operations. I remember now.

@sonarqubecloud

sonarqubecloud Bot commented Nov 13, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
15 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jacomago jacomago jacomago approved these changes
@shroffk shroffk Awaiting requested review from shroffk

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@georgweiss @jacomago